-
-
Notifications
You must be signed in to change notification settings - Fork 142
feat(database): support multiple databases in migrations and query builders #1267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
packages/database/src/Builder/QueryBuilders/CountQueryBuilder.php
Outdated
Show resolved
Hide resolved
|
@innocenzi I'd appreciate your input. I've left some clarification comments, and I'm aware there still are some todos |
innocenzi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty cool to see that we got to about the same conclusions, this looks a lot like my local changes 😼
I don't think having a UsesDatabase trait is that useful, its only purpose is to store a database variable with an awkward name that can conflict with internal properties.
I would go with a onDatabase method on the Query, and pass down Database instances from the query builders to the Query instance they create.
As for the BuildsQuery class, I think the name is fine as-is: it describes the intent of the interface very well, and the current QueryBuilder class is a builder class for building queries
No description provided.